Add Orion-Q: Qwen porting, diagnostics, and training prep on top of Orion - #2
Add Orion-Q: Qwen porting, diagnostics, and training prep on top of Orion#2LWH-9393 wants to merge 3 commits into
Conversation
mechramc
left a comment
mechramc
left a comment
There was a problem hiding this comment.
Thank you for sharing this. There is a substantial amount of useful Qwen porting and diagnostic work here, and the production tree builds cleanly against the latest upstream main. I also manually built all 33 added Qwen test executables, and the two self-contained frontend tests pass. I found a few integration blockers that should be addressed before the draft is promoted or merged, most importantly a shared model-config layout mismatch that regresses Orion's existing GPT-2 ANE graph generation. I have left focused comments with concrete paths forward.
| typedef struct { | ||
| int n_layer; | ||
| int n_head; | ||
| int n_kv_head; |
There was a problem hiding this comment.
Could we define OrionModelConfig once in a pure-C shared header, or at least keep these layouts byte-for-byte identical? core/ane_runtime.h places n_kv_head last, while this definition places it after n_head. Existing Objective-C paths include the runtime layout and pass it to compiler frontends built with this layout. I reproduced the result with GPT-2: the runtime passes d_model=768, but orion_frontend_gpt2_prefill_attn() reads the same slot as 64 and generates a 64-channel input graph. This regresses the existing --ane path even before Qwen is exercised. A single canonical struct plus a compile-time layout assertion would prevent this class of issue.
| double t_load = time_ms() - t0; | ||
| fprintf(stderr, "Weights loaded in %.1f ms\n", t_load); | ||
|
|
||
| OrionModelConfig cfg = kGPT2_124M; |
There was a problem hiding this comment.
The dynamic dimensions here are a useful start, but this command still loads OrionGPT2Weights, uses the GPT-2 tokenizer/EOS id, and calls only the GPT-2 prefill/decode functions. As a result, the Qwen execution paths added in this PR are not reachable through orion infer (or another registered CLI command). Could the PR add explicit model selection and dispatch through the Qwen manifest, tokenizer regex/special tokens, Qwen CPU path, and ANE/hybrid path? Alternatively, the documentation and PR scope should describe these as diagnostic scaffolding rather than a runnable Orion-Q integration.
| kernels/training/stories_train.m \ | ||
| kernels/training/stories_cpu_ops.m \ | ||
| kernels/training/qwen_lora_cpu_ops.m \ | ||
| kernels/training/qwen_lora_train.m \ |
There was a problem hiding this comment.
The Qwen production sources are now part of every build, but none of the 33 added test_qwen*.m programs are included in TEST_NAMES or a separate test target. They all compile when requested manually, and the two frontend tests run successfully, but make test never discovers them. Would you add a focused target (for example test-qwen) and include the self-contained tests in the default verification path, with model/ANE-dependent probes clearly separated? That would make the close-out claims reproducible for reviewers and CI.
|
|
||
| char path[256]; | ||
| snprintf(path, sizeof(path), "@model_path/layer%d/self_attn_q_proj.bin", layer); | ||
| int q_proj = orion_gb_linear(g, rms, "q_proj", d, d * 2, s, path, NULL); |
There was a problem hiding this comment.
Could this output dimension be derived from q_dim = n_head * head_dim rather than d_model? For the submitted 0.8B config, d_model=1024 while n_head * head_dim=2048; the gated Q projection therefore contains 2 * q_dim = 4096 channels, but this graph declares only 2 * d_model = 2048. The current frontend smoke test checks only that expected names appear in the MIL, so it does not catch the shape mismatch. An assertion against the converted manifest shape would be valuable here.
2 participants
Summary
This PR introduces Orion-Q, a Qwen-focused porting and diagnostics subset built on top of Orion.
It adds the code needed to run, validate, and prepare training flows for Qwen-family models inside Orion while keeping the relationship to upstream Orion explicit.
Scope
Included in this PR:
Explicitly excluded from this PR:
Why this is a subset, not a separate engine
Orion-Q is not meant to replace Orion.
It is a Qwen-specific extension and verification layer built on top of Orion.
Validation status
Local validation for the current Orion-Q scope includes:
Repo-local documents:
Review guide
Recommended review order:
Notes
This PR intentionally does not include generated artifacts or downstream track assets.
Those are kept out so the review stays focused on the Orion-Q code and validation surface.